Set namespsace for metrics to WATCH_NAMESPACE #2078
Conversation
Often the operator is in the same namespace as the things it's watching and this works okay. We deploy the operator to a different namespace. Anyway, I think this should be the watch namespace since that's where we're expecting the CRDs to be.
openshift-ci-robot
added
the
size/XS
|
Hi @mattmb. Thanks for your PR. I'm waiting for a operator-framework or openshift member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
openshift-ci-robot
added
the
needs-ok-to-test
| @@ -198,7 +198,7 @@ func serveCRMetrics(cfg *rest.Config) error { | |||
| return err | |||
| } | |||
| // Get the namespace the operator is currently deployed in. | |||
There was a problem hiding this comment.
Probably should update this comment too, since it's no longer accurate
|
/ok-to-test |
openshift-ci-robot
added
ok-to-test
There was a problem hiding this comment.
HI @mattmb,
Really thank you for your contribution. 🥇
However, IMHO we should NOT use the GetWatchNamespace() by default in the scaffold project. Note that this change just will just work if the WatchNamespace is a List of Namespaces since if the operator is namespace scoped then it means that it also should be the operator namespace. Then, if the operator is cluster scoped it will be "". For more info see the doc
Also, users are able to customize the scaffold as they wish but in order to keep the scaffold project and the common scenarios working then make sense the usage of the Operator Namespace.
PS.: Since it is passing in the tests it means that we are not checking the metrics with a cluster scope operator. It is something that we could improve. Tracked it here: #2084
c/c @fabianvf
|
Ah, interesting, thanks for the context. I wasn't aware you could have multiple watch namespaces... Some context on why I was trying to fix this. When I deployed a new operator with the latest version of the sdk the log was full with: I suspect the operator is still okay but just because we don't give the operators much permission in their own namespace the metrics code is failing. Given your context maybe a better solution is a toggle to make it easy to disable the metrics collection/serving? |
|
Hi @mattmb, Really thank you for your understanding. So, do you agree that would not be the best approach to move forward here? Are you ok with we close this PR? Also, regards any issue that you may have been facing with the new release could you please raise an issue for we check and are able to help you with instead of we check if here? |
|
Hi @mattmb, I am closing this one based on the comments made above. However, please feel free to re-open if you see that is required. Thank you for your contribution. |
… to Ansible/Helm operators to handle [multinamespace caching] (#2522) **Motivation for the change:** - Integration with OLM: See that OLM allows and config the MultiNamespace via the option `targetNamespaces` via the OperatorGroup. We are also in the olm commands setting these values in the WATCH-NAMESPACE EnvVar. - Address the requirements requested in tasks such as #2494, #2078, (which are a very common scenario in the channels: I as an operator dev, would like to deploy the operator in the nsA and WATCH the resources in the nsB and do not all cluster ), Closes #2361
Often the operator is in the same namespace as the things it's watching
and this works okay. We deploy the operator to a different namespace.
Anyway, I think this should be the watch namespace since that's where
we're expecting the CRDs to be.